sconstruct: correctly detect the language of xliff files when trying to skip English #17143
Conversation
seanbudd
added
the
conceptApproved
|
I have verified that the generated changes file has a section on 2025.1 (I also checked that the correct changes file was included with the launcher). |
|
It could go to beta with no harm. However, it is not technically
necessary as on beta the English xliff is always updated when ever the
markdown is. Thus the xliff will always produce the most up to date
markdown.
However, based on Sascha's comment this morning re scons cleaning the
markdown inappropriately, perhaps then it should.
I'm happy to rebase on beta.
|
michaelDCurran
force-pushed
the
noEnglishXliff
branch
from
d5033a6 to
48a497b
Compare
WalkthroughThe pull request modifies the method used to extract the language from the file path of localized Changes
Tip New review modelWe have updated our review workflow to use the Anthropic's Claude family of models. Please share any feedback in the discussion post on our Discord. New featuresWalkthrough comment now includes:
Notes:
Recent review detailsConfiguration used: .coderabbit.yml Files selected for processing (1)
Additional context usedPath-based instructions (1)
Additional comments not posted (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
This is now based on beta. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
sconstruct (1)
338-338: Incorrect directory structure assumption for xliff filesThe current implementation assumes that xliff files are directly under a directory named after the language. However, they are actually located under
user_docs/<lang>/filename.xliff. This discrepancy will cause the language extraction logic to fail, as it will extractuser_docsinstead of the actual language code.
- Update the language extraction logic to account for the additional directory level. Consider modifying the code to correctly extract the language from the path
user_docs/<lang>/filename.xliff.Analysis chain
Verify the directory structure assumption for xliff files.
The change to extract the language from the directory name is approved.
However, please verify that all the xliff files in the codebase follow the expected directory structure
<lang>/filename.xliff. If there are any xliff files that don't adhere to this structure, the language detection could fail.You can use the following script to verify the directory structure assumption:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all xliff files follow the directory structure `<lang>/filename.xliff` # Test: Find all xliff files and check their paths. Expect: All xliff files to be in a directory named after the language. find . -name "*.xliff" | while read -r xliffFile; do if [[ "$xliffFile" =~ \./([^/]+)/[^/]+\.xliff$ ]]; then echo "OK: $xliffFile" else echo "ERROR: $xliffFile does not follow the expected directory structure." fi doneLength of output: 10054
Link to issue number:
None.
Summary of the issue:
The NVDA build system creates html from markdown files. For any language other than English, these markdown files should be themselves generated from xliff files.
However, a rather strange and incorrect line of code caused this to also happen for English, as the line was not correctly detecting the language.
Description of user facing changes
English user documentation in alpha builds should be based on the very latest markdown files, rather than the last beta from the previous release.
Description of development approach
Fix the questionable line.
Testing strategy:
Known issues with pull request:
None known.
Code Review Checklist:
Summary by CodeRabbit
New Features
Bug Fixes